Show pdf in yml-enabled setup if this format enabled #2638
Conversation
def load_yaml_config(version): ... > checkout_path = version.project.checkout_path(version.slug) E AttributeError: 'NoneType' object has no attribute 'project' in test_has_pdf and test_has_pdf_with_pdf_build_disabled of the test_project.py.
| @@ -570,8 +572,15 @@ def has_aliases(self): | |||
| return self.aliases.exists() | |||
|
|
|||
| def has_pdf(self, version_slug=LATEST): | |||
| version = self.versions.filter(slug=version_slug).first() | |||
| if not self.enable_pdf_build: | |||
There was a problem hiding this comment.
We can probably just remove the initial check on this, and simply look for the file existing. I believe the issue is that we'd have PDF's sitting on disk that were stale, so we added this flag. Another option is to set this flag in the database based off the YAML file.
There was a problem hiding this comment.
@ericholscher, are you about this check, on L573? I think, I have tried this, that breaks tests.
There was a problem hiding this comment.
Yea, the tests would need to be changed, as it would be a change in how things work. Perhaps the easiest update would be to actually update the database for a project from the YAML file on build. I'd want @agjohnson's thoughts on that though, because it is a weird UX. Perhaps if we detect YAML we can hide the fields on the web UI, and update them from the YAML for consistency.
ericholscher
added
Needed: patch
|
@ericholscher, thank you for review and sorry for your time. I'll close this pr per #2362. |
This is a very early attempt to fix #2582.
It looks also, that documented yml syntax for "format" option was changed. This fix was tested in this repo and it doesn't work with .readthedocs.yml in my master branch, but it does if I change format configuration to:
(See yml file on another branch.)
Is the old syntax supported, or the docs should be adapted to reflect new syntax, documented in spec.rst?